Skip to content

Conversation

@MasterPtato
Copy link
Contributor

No description provided.

Copy link
Contributor Author

MasterPtato commented Jan 12, 2026

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more


How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@MasterPtato MasterPtato force-pushed the 01-12-fix_set_failure_reason_regardless_of_crash_policy branch from f7e33e7 to b613527 Compare January 12, 2026 19:11
@MasterPtato MasterPtato force-pushed the 01-09-feat_add_metrics_to_pb_guard branch from 9297e74 to 442651f Compare January 12, 2026 19:11
@claude
Copy link

claude bot commented Jan 12, 2026

PR Review: fix: set failure reason regardless of crash policy

Summary: This PR fixes an issue where the failure reason was not being set consistently across all crash policies.

Key Changes:

  1. Refactored failure reason handling - Moved SetFailureReason activity calls earlier
  2. Removed state.failure_reason reset - Failure reasons now persist across restarts
  3. Fixed workflow name handling - Added proper error handling for missing workflows

Code Quality:
Strengths - Correct precedence, better separation of concerns, defensive programming
Concerns - Line 1073 ctx.removed call unclear, state persistence needs review, needs comments

Recommendations:
HIGH - Clarify intent of ctx.removed at line 1073
MEDIUM - Consider clearing failure_reason on successful exit
TESTING - Add integration tests for crash policy scenarios

Assessment: Good fix addressing real bug. Improves code clarity. Some edge cases need clarification.

Recommendation: Approve with minor suggestions.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Jan 12, 2026

More templates

@rivetkit/cloudflare-workers

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/cloudflare-workers@3835

@rivetkit/db

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/db@3835

@rivetkit/framework-base

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/framework-base@3835

@rivetkit/next-js

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/next-js@3835

@rivetkit/react

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/react@3835

rivetkit

pnpm add https://pkg.pr.new/rivet-dev/rivet/rivetkit@3835

@rivetkit/sql-loader

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sql-loader@3835

@rivetkit/virtual-websocket

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/virtual-websocket@3835

@rivetkit/engine-runner

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner@3835

@rivetkit/engine-runner-protocol

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner-protocol@3835

commit: 97ace26

@MasterPtato MasterPtato force-pushed the 01-12-fix_set_failure_reason_regardless_of_crash_policy branch from b613527 to b4daf93 Compare January 13, 2026 00:27
@MasterPtato MasterPtato force-pushed the 01-09-feat_add_metrics_to_pb_guard branch 2 times, most recently from 7915d6f to 258ae43 Compare January 13, 2026 00:44
@MasterPtato MasterPtato force-pushed the 01-12-fix_set_failure_reason_regardless_of_crash_policy branch 2 times, most recently from 0b9ca2f to 1a08758 Compare January 13, 2026 01:01
@MasterPtato MasterPtato force-pushed the 01-09-feat_add_metrics_to_pb_guard branch 2 times, most recently from 42cfc6d to 506742d Compare January 13, 2026 01:29
@MasterPtato MasterPtato force-pushed the 01-12-fix_set_failure_reason_regardless_of_crash_policy branch from 1a08758 to 5b1c724 Compare January 13, 2026 01:29
@MasterPtato MasterPtato force-pushed the 01-09-feat_add_metrics_to_pb_guard branch from 506742d to ca4cbfc Compare January 14, 2026 02:05
@MasterPtato MasterPtato force-pushed the 01-12-fix_set_failure_reason_regardless_of_crash_policy branch from 5b1c724 to 8152462 Compare January 14, 2026 02:05
@MasterPtato MasterPtato force-pushed the 01-09-feat_add_metrics_to_pb_guard branch from ca4cbfc to 8102256 Compare January 14, 2026 03:06
@MasterPtato MasterPtato force-pushed the 01-12-fix_set_failure_reason_regardless_of_crash_policy branch from 8152462 to 82ed36d Compare January 14, 2026 03:06
@MasterPtato MasterPtato force-pushed the 01-12-fix_set_failure_reason_regardless_of_crash_policy branch from 82ed36d to f5a4f68 Compare January 14, 2026 19:47
@MasterPtato MasterPtato force-pushed the 01-09-feat_add_metrics_to_pb_guard branch 2 times, most recently from 4b907cb to dc985c0 Compare January 14, 2026 20:01
@MasterPtato MasterPtato force-pushed the 01-12-fix_set_failure_reason_regardless_of_crash_policy branch from f5a4f68 to 9e4ae6f Compare January 14, 2026 20:01
@MasterPtato MasterPtato force-pushed the 01-09-feat_add_metrics_to_pb_guard branch from dc985c0 to f3f7d72 Compare January 14, 2026 22:52
@MasterPtato MasterPtato force-pushed the 01-12-fix_set_failure_reason_regardless_of_crash_policy branch 2 times, most recently from 595d7b8 to 250667d Compare January 14, 2026 23:02
@MasterPtato MasterPtato force-pushed the 01-09-feat_add_metrics_to_pb_guard branch from f3f7d72 to ae1c40c Compare January 14, 2026 23:02
@MasterPtato MasterPtato force-pushed the 01-12-fix_set_failure_reason_regardless_of_crash_policy branch from 250667d to 980dd0a Compare January 14, 2026 23:07
@MasterPtato MasterPtato force-pushed the 01-09-feat_add_metrics_to_pb_guard branch from ae1c40c to 3da23f7 Compare January 14, 2026 23:07
@MasterPtato MasterPtato force-pushed the 01-12-fix_set_failure_reason_regardless_of_crash_policy branch from 980dd0a to 97ace26 Compare January 14, 2026 23:39
@MasterPtato MasterPtato force-pushed the 01-09-feat_add_metrics_to_pb_guard branch from 3da23f7 to aea6392 Compare January 14, 2026 23:39
@graphite-app
Copy link
Contributor

graphite-app bot commented Jan 14, 2026

Merge activity

  • Jan 14, 11:40 PM UTC: MasterPtato added this pull request to the Graphite merge queue.
  • Jan 14, 11:41 PM UTC: CI is running for this pull request on a draft pull request (#3908) due to your merge queue CI optimization settings.
  • Jan 14, 11:42 PM UTC: Merged by the Graphite merge queue via draft PR: #3908.

@graphite-app graphite-app bot closed this Jan 14, 2026
@graphite-app graphite-app bot deleted the 01-12-fix_set_failure_reason_regardless_of_crash_policy branch January 14, 2026 23:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants